Repository navigation
Add mssql-python as an optional DBAPI driver for MsSqlHook - #74539
RehanAhmad25 wants to merge 2 commits into
Conversation
bce8d0d to
dc2a3b0
Compare
| "host": "server.com", | ||
| "port": 1433, | ||
| "schema": "database_name", | ||
| "extra": {"dbapi_driver": "mssql_python", "TrustServerCertificate": "yes"} |
There was a problem hiding this comment.
Can you look at other db providers and see if there is a convention for this?
There was a problem hiding this comment.
Thanks for pointing this out. I went through the other DB providers to see what they do.
For picking a client library through an extra, MySQL is the only precedent, and it uses client (mysqlclient by default, or mysql-connector-python). JDBC uses driver_path and driver_class, and ODBC's driver is the ODBC driver name, so I went with dbapi_driver to avoid mixing it up with that. If you'd rather have client for consistency with MySQL, I'm happy to rename it.
On the docs, the other providers lead with a URI example and show the JSON form with extras nested under extra. I'll add a URI example next to the JSON one.
While checking, I also saw that the other hooks raise AirflowOptionalProviderFeatureException when an optional package is missing, so I'll switch the mssql-python import error to that.
| try: | ||
| mssql_python = import_module("mssql_python") | ||
| except ImportError as e: | ||
| raise ImportError( |
There was a problem hiding this comment.
I thought you said this should be AirflowOptionalProviderFeatureException?
closes: #74432
What
MsSqlHookcan now connect with Microsoft'smssql-pythondriver instead ofpymssql. The driver is chosen with a newdbapi_driverconnection extra (pymssqlormssql_python, case-insensitive).pymssqlstays the default, so existing connections behave exactly as before. An unsupported value raises aValueError.How it works
With
dbapi_driverset tomssql_python:get_conn()builds the connection withmssql_python.connect(...). The host and port are sent asServer=host,port, and the schema, login and password map toDatabase,UIDandPWD.Encrypt,TrustServerCertificate,Authentication) are passed to the driver as keyword arguments. The hook's own extras (dbapi_driver,sqlalchemy_scheme,placeholderand the otherDbApiHookformat extras) are not passed on, and empty values are dropped.mssql+mssqlpython.sqlalchemy_schemestill overrides it.get_uri()stripsdbapi_driverfrom the query string.?, because mssql-python does not accept a plain%s. Theplaceholderextra still overrides it.Rowobjects.autocommitproperty.mssql-pythonextra.pymssqlis now imported inside the pymssql code path, not at module level. It is still a required dependency of the provider. Making it optional would be a separate change.Where
providers/microsoft/mssql/src/airflow/providers/microsoft/mssql/hooks/mssql.py: the hook changesproviders/microsoft/mssql/tests/unit/microsoft/mssql/hooks/test_mssql.py: new testsproviders/microsoft/mssql/docs/connections/mssql.rst: new "Choosing the DBAPI driver" sectionproviders/microsoft/mssql/pyproject.toml: new optional extramssql-python(mssql-python>=1.15.0)providers/microsoft/mssql/docs/index.rst: extras table, updated by the repo's hookuv.lock: regenerated withuv 0.13.0for the new extra (addsmssql-pythonandmssql-python-odbc)Notes for reviewers
>=1.15.0as the floor because it is the version I checked the driver's behaviour against. Happy to loosen it if you prefer.Testing
I added 14 new test cases for driver selection and validation, scheme and placeholder defaults,
get_uri, the arguments passed toconnect, the missing-package error, autocommit and row conversion. The existing pymssql tests pass unchanged (35 passed in Breeze). The prek hooks, including ruff, mypy for providers and the provider.yaml check, pass locally.Both drivers are mocked in the tests, so I have not run this against a real SQL Server yet. The Microsoft Entra ID
Authenticationoptions in particular are untested, so a check on a real instance would be welcome.Related: #72000
Was generative AI tooling used to co-author this PR?
Generated-by: Claude (Anthropic) following the guidelines